MANTA-5522 Add lib/scope-schema.js as the single source of truth for bucket scope schema by cneira · Pull Request #11 · TritonDataCenter/node-mahi

cneira · 2026-04-28T14:11:48Z

Add a shared bucket scope validation module and two new endpoints for bucket scoped access keys:

cachePush : Push a key directly to mahi/authcache redis cache, the intent is that the newly created access key can be used right away and bypass UFDS replication delay.
scopeRevoke: Does the opposite, it inmediately deletes a key from the redis cache.

Co-Authored-By: Claude Opus 4.6 (1M context) noreply@anthropic.com

and MahiClient.prototype.scopeRevoke for immediate key deletion. Both are best-effort with warn-level logging on failure. Single source of truth for per-bucket access key scope constants, validation, and pattern matching. Exported via index.js as scopeSchema. Consumed by sdc-cloudapi directly and by manta-buckets-api via local copy. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Match the mahi server endpoint rename. The method name scopeRevoke() is unchanged (JS API, not REST path). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Parse and validate opts.scope using scope-schema before sending to the mahi server. Prevents malformed scope strings from poisoning the Redis cache and corrupting authorization decisions for affected access keys. Null/undefined scope (unrestricted key) is still accepted. AI-generated code. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Remove redundant local require('querystring') that shadowed the module-level qs variable (JSL warning). Strip trailing whitespace in client.js. Parenthesize typeof expressions in scope-schema.js per jsstyle. AI-generated code. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

danmcd

Checkpoint, and I shouldn't revisit here until I've completely looked at TritonDataCenter/mahi#30 .

travispaul

Please bump the version in package.json too

danmcd

Seems okay, but my too-narrow complaint holds here as well. Unsure why Claude emits it like this.

Cover validateScope shape rejection, envelope rejection, per-entry rejection, and success path. Smoke-test isValidBucketPattern, matchBucketPattern, and parseScope.

danmcd

Still too-few-columns in some cases, I think.

danmcd · 2026-05-04T20:25:53Z

+                    err: err,
+                    accesskeyid: accesskeyid
+                }, 'scopeRevoke: mahi call failed' +
+                    ' (non-fatal)');


For example, wouldn't this line and the prior on join together so it looks like:

if (err) { if (self.http.log) { self.http.log.warn({ err: err, accesskeyid: accesskeyid }, 'scopeRevoke: mahi call failed (non-fatal)'); }

danmcd · 2026-05-04T20:27:15Z

-        scope: scopeJson,
-        error: null
-    });
+    return ({valid: true, scope: scopeJson, error: null});


This one actually makes sense to stay on different lines. SOme of the other ones above are more what I had in mind.

danmcd · 2026-05-04T20:27:30Z

                error: 'scope: permissions[' + i +
-                    '].bucket: invalid characters' +
-                    ' or wildcard position'
+                    '].bucket: invalid characters' + ' or wildcard position'


Yeah, stuff like this is what I was talking about.

Also, and I forgot this 3 days ago, you can lose the + and just make it one const string here.

danmcd

Are you missing an export here, or am I missing something I don't understand?

Also more too-narrow code.

danmcd · 2026-05-07T13:16:31Z

                    accesskeyid: accesskeyid
-                }, 'scopeRevoke: mahi call failed' +
-                    ' (non-fatal)');
+                }, 'scopeRevoke: mahi call failed' + ' (non-fatal)');


Two const strings don't need to be +-ed.

}, 'scopeRevoke: mahi call failed (non-fatal));

danmcd · 2026-05-07T13:17:45Z

                error: 'scope: permissions[' + i +
-                    '].bucket: invalid characters' +
-                    ' or wildcard position'
+                    '].bucket: invalid characters' + ' or wildcard position'


Also, and I forgot this 3 days ago, you can lose the + and just make it one const string here.

danmcd · 2026-05-07T13:19:49Z

+                self.http.log.warn({
+                    err: err,
+                    accesskeyid: opts.accesskeyid
+                }, 'cachePush: mahi call failed' +


This and the following line can be squashed and made one constant string.

danmcd · 2026-05-07T13:20:23Z

+
 module.exports = {
    MahiClient: MahiClient,
    createClient: function (opts) {


Does cachePush need to be put here too?

This one (cachePush) I need an answer to, and then I'm likely good.

No, as it's defined as a prototype it will be available when a new MahiClient is instanciated.

danmcd

Cleared up my concerns here over on MANTA-5521. As with others, make sure you get @travispaul to sign-off on this as well.

travispaul

I'm good, I also noticed some const string concatenation but functionally it doesn't make much of a difference so not blocking on it.

cachePush wrote an access-key entry to a single mahi instance on create / update, and scopeRevoke deleted one on key removal. Both pretended to fan-out across the cache pool but only ever reached the single instance the caller resolved by DNS — the SDC mahi pool, never the Manta authcache pool that serves the S3 data plane. They were a single-instance optimisation on the SDC side that the sigv4 read-through (resolves cache misses against UFDS) made irrelevant. cloudapi stopped calling them. Drop both methods from MahiClient. Drop the scope-schema require they pulled in for scope validation (scopeSchema is still exported separately from index.js). External callers of node-mahi that consume cachePush / scopeRevoke will see a method-not-found at runtime; this is a breaking change in the next published version. === AI-METADATA === AI-Tool: Claude Code AI-Model: claude-opus-4-7 AI-Role: refactor / cleanup AI-Confidence: high Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

cneira and others added 5 commits April 22, 2026 20:11

fixup

62c2486

fix: rename /scope-revoke to /key-revoke endpoint path

99b3889

Match the mahi server endpoint rename. The method name scopeRevoke() is unchanged (JS API, not REST path). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

cneira changed the title ~~Allow bucket-scoped access keys~~ MANTA-5522 Add lib/scope-schema.js as the single source of truth for bucket scope schema Apr 28, 2026

cneira requested a review from a team April 28, 2026 16:49

danmcd reviewed Apr 28, 2026

View reviewed changes

Comment thread index.js

Comment thread lib/client.js Outdated

travispaul reviewed May 1, 2026

View reviewed changes

Comment thread lib/scope-schema.js

danmcd reviewed May 4, 2026

View reviewed changes

Comment thread lib/client.js Outdated

cneira added 3 commits May 4, 2026 13:47

Fix for format issue were lines were being truncated at 60 cols

47d6052

test: add unit tests for scope-schema validateScope

0e112fa

Cover validateScope shape rejection, envelope rejection, per-entry rejection, and success path. Smoke-test isValidBucketPattern, matchBucketPattern, and parseScope.

bump up package version

d8f6826

cneira requested review from danmcd and travispaul May 4, 2026 20:09

danmcd reviewed May 4, 2026

View reviewed changes

cneira added 2 commits May 6, 2026 11:33

Fix horizontal whitespace

b8d7800

Fix horizontal whitespace

d9eeb4d

cneira requested a review from danmcd May 6, 2026 18:51

Fix horizontal white space.

ed502d8

danmcd reviewed May 7, 2026

View reviewed changes

fix horizontal white space

5e85be3

cneira requested a review from danmcd May 7, 2026 15:19

danmcd previously approved these changes May 7, 2026

View reviewed changes

travispaul previously approved these changes May 11, 2026

View reviewed changes

cneira dismissed stale reviews from travispaul and danmcd via 012b503 May 12, 2026 22:40

cneira requested a review from danmcd May 12, 2026 22:57

cneira requested a review from travispaul May 12, 2026 22:57

Conversation

cneira commented Apr 28, 2026

Uh oh!

danmcd left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

travispaul left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

danmcd left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

danmcd left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

danmcd left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

danmcd left a comment

Choose a reason for hiding this comment

Uh oh!

travispaul left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants